Skip to content

Fix LogicalToBooleanRector: parenthesize assignments nested under unary/binary operators#8184

Merged
TomasVotruba merged 1 commit into
mainfrom
fix-logical-to-boolean-nested-assign-parens
Jul 14, 2026
Merged

Fix LogicalToBooleanRector: parenthesize assignments nested under unary/binary operators#8184
TomasVotruba merged 1 commit into
mainfrom
fix-logical-to-boolean-nested-assign-parens

Conversation

@TomasVotruba

@TomasVotruba TomasVotruba commented Jul 13, 2026

Copy link
Copy Markdown
Member

Fixes rectorphp/rector#9807 and rectorphp/rector#9808

LogicalToBooleanRector swaps and/or (low precedence) for &&/|| (high precedence). An assignment operand must then be wrapped in parentheses to keep the original semantics. This worked for a direct assignment operand, but not when the assignment was nested one level down — under a unary operator (!, ~, +, -, @, casts) or as an operand of a binary operator when it was a compound assignment (+=, -=, …). The result changed runtime behaviour, and ~$x = ... even produced a fatal TypeError.

Before / after

-!$x = 42 and true;        // !($x = 42)
+!$x = 42 && true;         // BUG: parses as !$x = (42 && true)
// input
$x = 0;
2 * $x += 42 and true;     // 2 * ($x += 42) → true, $x is 42

// before (broken)
2 * $x += 42 && true;      // 2 instead of true, $x is 1

// after (fixed)
2 * ($x += 42) && true;    // true, $x is 42

All reported forms now print correctly:

!($y = 42) && true;
~($w = 42) && true;
+($v = 42) && true;
-($z = 42) && true;
@($u = 42) && true;
(int) ($t = 42) && true;
2 * ($x += 42) && true;

Fix

Handled in LogicalToBooleanRector, not the printer. When the operator is swapped, each operand is walked and any nested assignment (Assign / AssignOp / AssignRef) — reached through a unary operator (! ~ + - @, casts) or through binary operators — is marked to reprint. The pretty-printer then re-adds the precedence parentheses via its existing generic mechanism.

This keeps the logic on the LogicalOr/LogicalAnd nodes the rule already subscribes to, instead of running per-node bracket checks in BetterStandardPrinter on every print.

Covered by 5 fixtures. Existing printer + LogicalToBooleanRector tests stay green.

@TomasVotruba TomasVotruba force-pushed the fix-logical-to-boolean-nested-assign-parens branch from 3d1cd0f to 2fd1aa5 Compare July 14, 2026 07:32
…ators in LogicalToBooleanRector (#9807, #9808)
@TomasVotruba TomasVotruba force-pushed the fix-logical-to-boolean-nested-assign-parens branch from 2fd1aa5 to 03af320 Compare July 14, 2026 08:08
@TomasVotruba TomasVotruba merged commit ce9583f into main Jul 14, 2026
65 checks passed
@TomasVotruba TomasVotruba deleted the fix-logical-to-boolean-nested-assign-parens branch July 14, 2026 08:17
@TomasVotruba

Copy link
Copy Markdown
Member Author

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect behavior of LogicalToBooleanRector: missing parentheses around assignments when prefixed by unary operator

1 participant